Added support for style selector parsing #619
Conversation
🦋 Changeset detectedLatest commit: 243f4c4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
marekdedic
force-pushed
the
selector-node-loc-fixing
branch
7 times, most recently
from
47e8c2b to
385e9ec
Compare
Pull Request Test Coverage Report for Build 12330056473Details
💛 - Coveralls |
marekdedic
force-pushed
the
selector-node-loc-fixing
branch
from
ee0c676 to
53e9124
Compare
…or locations to eslint locations
marekdedic
force-pushed
the
selector-node-loc-fixing
branch
from
53e9124 to
243f4c4
Compare
There was a problem hiding this comment.
I added 1 more comment🙏 After solve this comment, I will merge this. @marekdedic
| export function styleSelectorNodeLoc( | ||
| node: SelectorNode, | ||
| ): Partial<SourceLocation> { | ||
| return { | ||
| start: | ||
| node.source?.start !== undefined | ||
| ? { | ||
| line: node.source.start.line, | ||
| column: node.source.start.column - 1, | ||
| } | ||
| : undefined, | ||
| end: node.source?.end, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Why don't you do this in fixSelectorNodeLocation? I thought the loc information could always be handled in ESLint style because this program runs on ESLint.
There was a problem hiding this comment.
Yeah, I'm following the same approach we agreed on in #340 (comment) - the TL;DR is that PostCSS AST locations are different than ESLint AST locations - one of them starts counting columns from 0 and other from 1, one of them has the end point at the last character while the other has it point at the first character after the end... So we agreed we would recalculate the positions to be relative to the whole file instead of being relative to the selector, but we would keep the PostCSS semantics in the PostCSS AST. Only when converting from PostCSS AST to an ESLint SourceLocation would we convert this to the ESLint way of doing things.
There was a problem hiding this comment.
Of course this is debatable (there is no universally "right" solution IMO), but I'd strongly suggest doing the selector parsing in the same way as the style node parsing. So if we were to change this one, we'd need to change the other one too.
Needed for sveltejs/eslint-plugin-svelte#898